Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the schema_search_path in prepared statements. #3232

Merged
merged 2 commits into from Oct 7, 2011
Merged

Use the schema_search_path in prepared statements. #3232

merged 2 commits into from Oct 7, 2011

Conversation

Juanmcuello
Copy link
Contributor

I'm using postgreSQL with multiple schemas.

Rails 3.1 uses prepared statements and does not take into account the 'schema_search_path'. This is a big problem because the statement is prepared once and then the same prepared statement is executed many times no matter if you changed the schema, i.e, the prepared statement is executed ALWAYS for the same schema, the one set when the statement was prepared.

This patch adds the schema search to the sql key to allow the use of prepared statements when changing schemas in postgreSQL,

Discussion here:
https://groups.google.com/forum/#!msg/rubyonrails-core/r2VOKcyUyrs/JQj4MB_lhRMJ

To allow the use of prepared statements when changing schemas in
postgres, the schema search path is added to the sql key.
@tenderlove
Copy link
Member

Can you write a test for this? We have schema tests for PG in the active record suite.

Add tests for prepared statements with multiple schemas in
postgreSQL.
@Juanmcuello
Copy link
Contributor Author

Done!

@tenderlove
Copy link
Member

Great! Thank you! ❤️

tenderlove added a commit that referenced this pull request Oct 7, 2011
Use the schema_search_path in prepared statements.
@tenderlove tenderlove merged commit 3088d23 into rails:master Oct 7, 2011
@kevinsapp
Copy link

Thank you for creating this patch! This problem was driving me crazy until I found your post on the topic. Until this patch is "officially released" in an upcoming version of Rails, I'm working around the problem by calling

ActiveRecord::Base.connection.clear_cache! # HACK: Workaround

...just before setting a new search path. I'm not familiar with the Rails release process, but I hope this fix is included in the next version.

@Juanmcuello
Copy link
Contributor Author

I hope so too! Meanwhile, I'm using the same workaround.

Good to know the patch works for you. :)

@Juanmcuello
Copy link
Contributor Author

@jonleighton, any chance to include this in next 3.1.2 release?

@jonleighton
Copy link
Member

The RC has already gone out but I am pretty sure this is in there - check the CHANGELOG though.

@jonleighton
Copy link
Member

Actually, doesn't look like it is in the release, sorry.

It's too late now to put it in I'm afraid. If you'd like it in the next release, please submit a pull request with a backport. Thanks.

@Juanmcuello
Copy link
Contributor Author

After digging a while into git history, I think this is what happened:

In master branch, part of the code introduced by cfc95d8 (included in this pull request) was later modified in 6a28c51.

Instead of porting the two commits to 3.1.2, only the last commit was ported. It was not ported exactly as it was, it was modified instead, and the changes included code from the first commit (???). This changes resulted in commit 818d285.

This pull request also includes tests, but they were not ported to 3.1.2 at all.

So, the problem solved by this pull request seems to be already solved in 3.1.2 by 818d285. The only missing part are tests.

Do I create a pull request to back-port tests?

@jonleighton
Copy link
Member

Ok, thanks for looking into it :)

If you can create a PR with the tests and a CHANGELOG entry (under 3.1.2) that would be great. Thanks.

jonleighton added a commit that referenced this pull request Nov 15, 2011
jonleighton added a commit that referenced this pull request Nov 15, 2011
@Juanmcuello Juanmcuello deleted the pg_prepared_statements branch November 16, 2014 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants